Page Scroll Goals #5029

RobertJoonas · 2025-01-29T12:23:11Z

Required migration steps

Migrating from the old unique index to the new one is a bit tricky, requiring multiple steps:

Migration: Add scroll_threshold column to Goal and add the new unique index. Page Scroll Goals: migration step 1 #5033
Minimal schema change: Make Ecto aware of the new unique_constraint. Page Scroll Goals: migration step 2 #5034
Migration: Drop the old index. Page Scroll Goals: migration step 3 #5036

Once the old index is dropped, this PR is ready to go.

Changes

This PR implements page scroll goals.

A page scroll goal can be created by simply adding a value into an optional scroll_threshold field when creating a pageview goal:

A user can filter and break down page scroll goals like any other goals.

TODO: figure out what to do with the total conversions metric

Tests

Automated tests have been added

Changelog

This PR does not make a user-facing change

Documentation

This change does not need a documentation update

Dark mode

This PR does not change the UI

priv/repo/migrations/20250128161815_add_scroll_threshold_to_goals.exs

macobo

Some initial thoughts on the PR. Will dig into the query side of this in-depth in another pass.

lib/plausible_web/live/goal_settings/form.ex

lib/plausible/stats/sql/query_builder.ex

test/plausible_web/controllers/api/external_stats_controller/query_test.exs

test/plausible_web/controllers/api/stats_controller/conversions_test.exs

lib/plausible_web/live/goal_settings/form.ex

lib/plausible/goals/filters.ex

lib/plausible/stats/sql/query_builder.ex

lib/plausible/stats/sql/expression.ex

test/plausible_web/controllers/api/external_stats_controller/query_imported_test.exs

macobo · 2025-02-03T13:56:30Z

test/plausible_web/controllers/api/external_stats_controller/query_goal_dimension_test.exs

    end
  end
+
+  describe "page scroll goals" do


I think we're still missing a test showing the behavior with imported data + conversion rate + goal group by.

lib/plausible/stats/sql/expression.ex

Co-authored-by: Karl-Aksel Puulmann <[email protected]>

RobertJoonas · 2025-02-03T14:09:06Z

I think we're still missing a test showing the behavior with imported data + conversion rate + goal group by.

@macobo test/plausible_web/controllers/api/external_stats_controller/query_imported_test.exs:1468 is testing it. If you meant to say imported data + group conversion rate + goal group by - that's not possible with imported data, as mixing multiple dimensions will make the query ineligible for imported data.

github-actions · 2025-02-03T14:16:51Z

Preview environment👷🏼‍♀️🏗️
PR-5029

macobo

Would be happy if the testing nit were resolved but other than that looks great!

RobertJoonas added 4 commits January 29, 2025 13:15

migration: add scroll_threshold to goals

ecba7d7

update goal schema

ecbdc8e

setup simple UI for creating scroll goals

d5631de

add ability to filter and breakdown scroll goals

58e930f

RobertJoonas requested a review from macobo January 29, 2025 12:23

macobo reviewed Jan 29, 2025

View reviewed changes

priv/repo/migrations/20250128161815_add_scroll_threshold_to_goals.exs Show resolved Hide resolved

macobo reviewed Jan 29, 2025

View reviewed changes

priv/repo/migrations/20250128161815_add_scroll_threshold_to_goals.exs Show resolved Hide resolved

macobo reviewed Jan 29, 2025

View reviewed changes

fix goals form tests

6c30e0f

macobo reviewed Jan 29, 2025

View reviewed changes

lib/plausible/goals/filters.ex Outdated Show resolved Hide resolved

add valiation for page path exists

7efca92

macobo reviewed Jan 29, 2025

View reviewed changes

lib/plausible/stats/sql/query_builder.ex Outdated Show resolved Hide resolved

RobertJoonas added 4 commits January 29, 2025 14:40

move todo comments to expression.ex

d19fe65

move tests

1848d44

make it clear that scroll_threshold is optional

5bef111

avoid calling Plausible.Goal.type() too many times

ec5f801

macobo reviewed Jan 29, 2025

View reviewed changes

lib/plausible/stats/sql/expression.ex Show resolved Hide resolved

RobertJoonas added 2 commits January 29, 2025 15:55

do not consider 255 scroll depth a conversion

1ac92fb

migration: add scroll_threshold to goals

08883b0

RobertJoonas changed the title ~~Page scroll goals~~ Page Scroll Goals Jan 30, 2025

RobertJoonas mentioned this pull request Jan 30, 2025

Page Scroll Goals: migration step 1 #5033

Merged

RobertJoonas and others added 3 commits January 30, 2025 12:13

do not drop the old index yet

0439793

More efficient goals join again

dbaf9b3

Refactor: move goals stats code explicitly under Stats.Goals module

8f28b28

macobo force-pushed the page-scroll-goals branch 2 times, most recently from 9978623 to 3c63b79 Compare January 30, 2025 11:35

Move code under Plausible.Stats.Goals

7e2e780

macobo force-pushed the page-scroll-goals branch from 3c63b79 to 7e2e780 Compare January 30, 2025 11:39

254 -> 100

ebf0dfb

macobo force-pushed the page-scroll-goals branch from da3f88a to ebf0dfb Compare January 30, 2025 11:52

adjust test to test what it claims to

3d0a131

This was referenced Jan 30, 2025

Page Scroll Goals: migration step 3 #5036

Merged

Page Scroll Goals: migration step 2 #5034

Merged

RobertJoonas and others added 7 commits January 30, 2025 14:41

mix format

72e2934

add migration

d17893d

consider imported query unsupported when page scroll goal filter

890690e

add missing tests

f8914e6

pattern match imported argument

9c51ba8

silence credo

0becb4a

Merge branch 'master' into page-scroll-goals

0da6616

RobertJoonas requested a review from macobo February 3, 2025 13:23

Merge branch 'master' into remove-goals-page-path-unique-index-migration

537d22a

macobo reviewed Feb 3, 2025

View reviewed changes

lib/plausible/stats/sql/expression.ex Outdated Show resolved Hide resolved

Update lib/plausible/stats/sql/expression.ex

1896791

Co-authored-by: Karl-Aksel Puulmann <[email protected]>

Merge with migration step 3 (PR #5036)

a8cda3c

RobertJoonas added the preview label Feb 3, 2025

macobo approved these changes Feb 3, 2025

View reviewed changes

RobertJoonas added 2 commits February 4, 2025 11:27

use site_imports populated in test setup

2ab0ff9

Merge remote-tracking branch 'origin/master' into page-scroll-goals

585ac1d

RobertJoonas added this pull request to the merge queue Feb 4, 2025

Merged via the queue into master with commit 4d05245 Feb 4, 2025
8 checks passed

RobertJoonas deleted the page-scroll-goals branch February 4, 2025 10:49

This was referenced Feb 9, 2025

Page Scroll Goals: migration step 3 dev-shahh/analytics#17

Open

Page Scroll Goals: migration step 2 dev-shahh/analytics#19

Open

Page Scroll Goals: migration step 1 dev-shahh/analytics#20

Open

macobo mentioned this pull request Feb 18, 2025

Perf: Speed up conversions query #5084

Merged

Uh oh!

Page Scroll Goals #5029

Page Scroll Goals #5029

Uh oh!

Conversation

RobertJoonas commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Required migration steps

Changes

Tests

Changelog

Documentation

Dark mode

Uh oh!

Uh oh!

Uh oh!

macobo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

macobo Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RobertJoonas commented Feb 3, 2025

Uh oh!

github-actions bot commented Feb 3, 2025

Uh oh!

macobo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

RobertJoonas commented Jan 29, 2025 •

edited

Loading

macobo Feb 3, 2025 •

edited

Loading